Skip to content

[tooling]: Enforce in CI generation of gen.go files #1456

Open
the-glu wants to merge 1 commit into
interuss:masterfrom
Orbitalize:enforce_generation
Open

[tooling]: Enforce in CI generation of gen.go files #1456
the-glu wants to merge 1 commit into
interuss:masterfrom
Orbitalize:enforce_generation

Conversation

@the-glu
Copy link
Copy Markdown
Contributor

@the-glu the-glu commented May 6, 2026

This PR follows #1455

It does enforce that *.gen.go files have been generated and don't change.

Minor side fix on docker run command, that set -it for no reason and that make it failing in ci. Changed to --rm because locally it's building up useless containers quickly.

@the-glu the-glu force-pushed the enforce_generation branch 5 times, most recently from ea8c4d6 to 5f02bae Compare May 6, 2026 14:03
Copy link
Copy Markdown
Contributor

@mickmis mickmis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo comment

Comment thread .github/workflows/ci.yml Outdated
- name: Check migrations
run: ./test/migrations/up_and_down.sh

generated-files:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like this belongs to repository hygiene. Also put the script in the makefile, and delete the hashfiles?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid having it in the Makefile, because it's going to generate the files, and therefor change the state of the repo/files with a check. Is it really ok to do so?

(And locally / in the Makefile, running the check will fix it, so that not really a check ^^')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if there is a change, we are a in a dirty state and should fail. BTW we could make that explicit by not using hashes but checking if there is any local change (e.g. git diff --exit-code).
So what about doing:

make apis
git diff --exit-code

Copy link
Copy Markdown
Contributor

@mickmis mickmis May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And also move it to repo hygiene (i.e. a step in hygiene-tests)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it but

  • It will check more than generated files
  • It may be influenced by previous steps generating some change for some reasons

so I don't think that the "correctest" approach.

@the-glu the-glu force-pushed the enforce_generation branch 3 times, most recently from 1ff87ab to 28dda10 Compare May 13, 2026 07:02
@the-glu the-glu force-pushed the enforce_generation branch from 28dda10 to 226ab68 Compare May 13, 2026 13:45
@the-glu the-glu requested a review from mickmis May 13, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants